Add MigrateSurefireFailsafeForkMode recipe by jonico · Pull Request #1306 · openrewrite/rewrite-migrate-java

jonico · 2026-10-18T15:43:52Z

Fixes item 1 of #1227 (Surefire forkMode -> forkCount/reuseForks).

Per discussion on #1227, this lives here next to the other Surefire/Failsafe recipes (AddSurefireFailsafeArgLine, AddMockitoJavaAgentToMavenSurefirePlugin) rather than in rewrite-static-analysis, for the same reason those do: it's Maven-plugin-configuration surgery, not a static-analysis/code-style concern.

What it does

Surefire 3 dropped support for the deprecated forkMode parameter — it's silently ignored rather than honored, so a build that still sets it gets different (and undocumented) fork behaviour after upgrading, with no warning. This recipe migrates it to the documented modern equivalent, per the Surefire fork options docs:

`forkMode`	becomes
`never`	`forkCount=0`
`once`	`forkCount=1`, `reuseForks=true`
`always`	`forkCount=1`, `reuseForks=false`
`perthread` (with `threadCount=N`)	`forkCount=N`, `reuseForks=false`

Applies to both maven-surefire-plugin and maven-failsafe-plugin. A perthread config with no paired threadCount, or any value outside those four, is left untouched rather than guessed at. If forkCount/reuseForks are already present alongside forkMode, only the now-dead forkMode tag is removed (Surefire 3 already ignores it in that case).

Validation

I wasn't able to run the module's own Gradle test suite locally — compileJava here needs rewrite-static-analysis, and that specific artifact returns 403 Forbidden against CodeGenome with my account regardless of version (release or snapshot), while core rewrite-core/rewrite-java/rewrite-maven resolve fine. Account-level entitlement gap, not a build config issue, so I validated independently instead: a minimal standalone Gradle project depending only on the public rewrite-core/rewrite-xml/rewrite-maven/rewrite-test artifacts (8.56.1, Maven Central), with just these two new files copied in. All 8 tests pass there:

alwaysOnFailsafe()                                PASSED
perthreadWithoutThreadCountIsLeftUntouched()      PASSED
redundantForkModeAlongsideForkCountIsRemoved()    PASSED
once()                                            PASSED
never()                                           PASSED
unrelatedPluginIsLeftUntouched()                  PASSED
unrecognizedValueIsLeftUntouched()                PASSED
perthreadWithThreadCount()                        PASSED

8 tests, 0 failures, 0 errors

Happy to split this into its own tracking issue if that's still wanted, or adjust the mapping/placement if I've read the discussion wrong.

Generated with Claude Code

Surefire 3 silently ignores the deprecated `forkMode` parameter (`once`/`always`/`never`/`perthread`) instead of honoring it, changing fork behaviour out from under any build that still sets it (openrewrite#1227). Migrates each value to its documented modern equivalent: - never -> forkCount=0 - once -> forkCount=1, reuseForks=true - always -> forkCount=1, reuseForks=false - perthread (with threadCount=N) -> forkCount=N, reuseForks=false Applies to both maven-surefire-plugin and maven-failsafe-plugin. A perthread configuration with no threadCount, or any unrecognized value, is left untouched rather than guessed at. A forkMode alongside an already-present forkCount/reuseForks has only the now-dead forkMode tag removed, since Surefire 3 already ignores it in that case. Co-Authored-By: Claude <noreply@anthropic.com>

CI's recipeCsvValidateCompleteness check requires every recipe in the marketplace environment to be listed in the CSV. Co-Authored-By: Claude <noreply@anthropic.com>

jonico · 2026-10-18T16:08:54Z

Both CI failures here are pre-existing and unrelated to this change:

ExtractExplicitConstructorInvocationArgumentsTest (13 cases) — already tracked in Preserve Java 25 constructor extraction statements #1206, which documents the same recipe producing broken code (references to undeclared variables) specifically on Java 25; that test class is gated to run only on Java 25+, which is why it surfaces in the test (Java 25) job here.
EhcacheJavaxToJakartaTest > importedBomManagedDependencyGetsExplicitJakartaVersion() — unrelated recipe, untouched by this diff.

MigrateSurefireFailsafeForkModeTest's own 8 tests pass in both CI runs. Should go green once #1206 lands or on a rerun against a non-Java-25 job.

…safe-forkmode

…Forks` Up to 3.0.0-M7, Surefire's `convertDeprecatedForkMode` overwrote `forkCount` and `reuseForks` with the values implied by a non-`once` `forkMode`. So a build with `forkMode=always` and `forkCount=2` really ran with one fork per test class. Only removing `forkMode` in that case would switch the build to two reused forks, which is the silent change this recipe sets out to prevent. The implied values now overwrite any existing ones; `once` and the `perthread` `reuseForks` default only fill in what is missing.

… `pertest` aliases

…va25`

timtebeek · 2026-10-23T20:55:34Z

Thanks so much for the larger summary, and contributing this fix here; I've triggered some automated scans and had a look through myself; I'd say we're close to getting this in. 🙏🏻

timtebeek

Thanks a lot for picking this up @jonico, and for the detailed write-up on #1227! I've pushed a few changes; listing them here mostly for any next PRs:

forkMode now takes precedence over an existing forkCount / reuseForks. Up to 3.0.0-M7, Surefire's convertDeprecatedForkMode overwrote those two with the values implied by a non-once forkMode. So forkMode=always next to forkCount=2 really ran one fork per test class. Only dropping forkMode there would have introduced exactly the silent change this recipe sets out to prevent. The values implied by always/never/perthread now overwrite existing ones; once and the perthread reuseForks default only fill in what is missing.
Also migrate forkMode inside <executions><execution><configuration>, which is common for Failsafe, plus the legacy none and pertest aliases Surefire still accepted (they map to never and always). pertest was used as the "unrecognized" value in the tests, so that test now uses a truly invalid value.
Run it as part of UpgradePluginsForJava25, right after the Surefire/Failsafe bump to 3.5.x. Otherwise folks would have to seek out this recipe and run it on their own, which isn't realistic at scale. A test there covers the combination.
Tests: dropped the mavenProject(..) wrapper and the unused <parallel> from the inputs, and grouped the no-change cases in a @Nested class NoChange.
Regenerated recipes.csv with ./gradlew recipeCsvGenerate for the new description and composite counts.

I left two questions inline where I'd like your take.

timtebeek · 2026-10-23T20:57:15Z

+                if (!threadCount.isPresent()) {
+                    return config;
+                }
+                c = setIfAbsent(set(config, forkMode, "forkCount", threadCount.get()), forkMode, "reuseForks", "false");


Curious what you think here: the docs table maps perthread to reuseForks=false, but from 2.14 up to 3.0.0-M7 convertDeprecatedForkMode only set forkCount = threadCount and left reuseForks at its default of true. So for anyone coming from 2.14+ (most builds today, I think), adding reuseForks=false changes behaviour, and test runs could get noticeably slower with a fresh JVM per class. Would it not make more sense to leave reuseForks alone for perthread, to match what those builds actually ran? I kept your mapping as-is for now, only making it not overwrite an explicit reuseForks.

@timtebeek I'll go with leaving reuseForks untouched for perthread as it's consistent with the rest of the recipe already preferring "what actually ran" over "what's written down" (e.g. forkMode overriding an existing forkCount/reuseForks, which mirrors the old convertDeprecatedForkMode behavior directly). Pushed the change plus a test proving an explicit existing reuseForks still survives untouched.

timtebeek · 2026-10-23T20:57:15Z

+        };
+    }
+
+    private static boolean isSurefireOrFailsafePlugin(Cursor cursor) {


I wonder if we should guard against plugin versions before 2.14, which don't know forkCount/reuseForks at all. That includes a plugin without an explicit <version> on Maven before 3.9, where the default lifecycle binds Surefire 2.12.4. There we'd remove a forkMode that was still honoured, and the new parameters would be ignored. Within UpgradePluginsForJava25 the version gets bumped to 3.5.x right before, but only when one is declared. (Not checked how common this is.) Perhaps only migrate when the effective plugin version is absent from the POM or at least 2.14? Not a blocker from my side, but curious what you think.

@timtebeek This is what happened in the real migration #1227 is about. My project ccfmaster's original pom had:

<artifactId>maven-surefire-plugin</artifactId> <version>2.8.1</version> ... <forkMode>always</forkMode>

2.8.1 predates 2.14 by several years. If this recipe had run standalone against that pom, without the version bump happening in the same pass, it would have deleted a forkMode that 2.8.1 genuinely honored and replaced it with forkCount/reuseForks that 2.8.1 silently ignores. The reason why it worked for me is that the version bump (2.8.1 → 3.5.2) and the forkMode fix happened together, in the same change.

Your proposed condition (migrate when the effective version is absent or at least 2.14, skip otherwise) handles this real case correctly: My original pom had an explicit pre-2.14 version, so it correctly falls into "skip."

I pushed the changes along your proposal as well added test cases and ran successfully with my project, let me know what you think.

Surefire's own convertDeprecatedForkMode (2.14 up to 3.0.0-M7) left reuseForks at its default of true for forkMode=perthread, never writing false as the documented mapping describes. Following the docs instead of that actual behaviour would silently make real builds' tests slower (fresh JVM per class) on upgrade - the exact kind of change this recipe exists to prevent. Co-Authored-By: Claude <noreply@anthropic.com>

forkCount/reuseForks were introduced in Surefire 2.14; an explicit older version does not recognize them and would silently ignore the replacement while the forkMode it understood is removed. An absent version is still treated as safe to migrate. Verified against ccfmaster's actual pre-migration pom (openrewrite#1227): maven-surefire-plugin 2.8.1 with forkMode=always. Without this guard, the recipe would have deleted a forkMode that 2.8.1 honored and added forkCount/reuseForks that 2.8.1 silently ignores - the exact regression this recipe exists to prevent. Co-Authored-By: Claude <noreply@anthropic.com>

…safe-forkmode # Conflicts: # src/main/resources/META-INF/rewrite/recipes.csv

…safe-forkmode # Conflicts: # src/main/resources/META-INF/rewrite/java-version-25.yml # src/main/resources/META-INF/rewrite/recipes.csv

jonico · 2026-11-04T15:44:49Z

Rebased onto current main to clear the merge conflict (unrelated churn in java-version-25.yml/recipes.csv from other merged PRs). Also addressed both inline questions from the review:

Left reuseForks untouched for perthread rather than setting false, matching what Surefire's own convertDeprecatedForkMode (2.14–3.0.0-M7) actually did, not the documented table where the two disagree.
Added the version guard: skips migration when the plugin has an explicit version below 2.14 (which predates forkCount/reuseForks and would silently ignore them) or an unparseable one, migrates when absent or ≥2.14. Verified against ccfmaster's actual pre-migration pom from Feedback from a 152k-line Java 8 to 25 migration: the silent behavioural changes that cost the most, and which have no recipe #1227 (surefire 2.8.1 + forkMode=always) — confirms it's now left untouched there.

@timtebeek whenever you get a chance to take another look 🙏

…safe-forkmode

jonico · 2026-11-11T17:26:22Z

Heads up: main's churn in java-version-25.yml/recipes.csv reintroduced the merge conflict since my last update — rebased onto current main again, tests still green. Mergeable as of now, whenever you have a chance @timtebeek 🙏

moderne-meeseeks Bot added this to OpenRewrite Oct 18, 2026

moderne-meeseeks Bot assigned jonico Oct 18, 2026

github-project-automation Bot moved this to In Progress in OpenRewrite Oct 18, 2026

jonico mentioned this pull request Oct 18, 2026

Feedback from a 152k-line Java 8 to 25 migration: the silent behavioural changes that cost the most, and which have no recipe #1227

Open

Add MigrateSurefireFailsafeForkMode to recipes.csv

a3d96f0

CI's recipeCsvValidateCompleteness check requires every recipe in the marketplace environment to be listed in the CSV. Co-Authored-By: Claude <noreply@anthropic.com>

timtebeek self-requested a review October 23, 2026 20:32

timtebeek added 5 commits October 23, 2026 22:35

Merge remote-tracking branch 'origin/main' into migrate-surefire-fail…

901b7b0

…safe-forkmode

Also migrate forkMode in execution configuration and its none and…

c4b7749

… `pertest` aliases

Use minimal test inputs and group no-change cases in a nested test class

deb8850

Run MigrateSurefireFailsafeForkMode as part of `UpgradePluginsForJa…

b7a9aa6

…va25`

timtebeek reviewed Oct 23, 2026

View reviewed changes

jonico and others added 7 commits October 24, 2026 01:30

Merge remote-tracking branch 'origin/main' into migrate-surefire-fail…

46d4295

…safe-forkmode # Conflicts: # src/main/resources/META-INF/rewrite/recipes.csv

Merge remote-tracking branch 'origin/main' into migrate-surefire-fail…

257b2d9

…safe-forkmode # Conflicts: # src/main/resources/META-INF/rewrite/recipes.csv

Merge remote-tracking branch 'origin/main' into migrate-surefire-fail…

3d7d820

…safe-forkmode # Conflicts: # src/main/resources/META-INF/rewrite/recipes.csv

Merge remote-tracking branch 'origin/main' into migrate-surefire-fail…

a5f4ab0

…safe-forkmode # Conflicts: # src/main/resources/META-INF/rewrite/recipes.csv

Merge remote-tracking branch 'origin/main' into migrate-surefire-fail…

00c46cc

…safe-forkmode # Conflicts: # src/main/resources/META-INF/rewrite/java-version-25.yml # src/main/resources/META-INF/rewrite/recipes.csv

Merge remote-tracking branch 'origin/main' into migrate-surefire-fail…

e93c438

…safe-forkmode

Navigation Menu

Repository navigation

Add MigrateSurefireFailsafeForkMode recipe - #1306

Add MigrateSurefireFailsafeForkMode recipe#1306
jonico wants to merge 15 commits into
openrewrite:mainfrom
jonico:migrate-surefire-failsafe-forkmode

jonico commented Oct 18, 2026

Uh oh!

jonico commented Oct 18, 2026

Uh oh!

timtebeek commented Oct 23, 2026

Uh oh!

timtebeek left a comment

Uh oh!

timtebeek Oct 23, 2026

Uh oh!

jonico Oct 23, 2026

Uh oh!

timtebeek Oct 23, 2026

Uh oh!

jonico Oct 23, 2026

Uh oh!

jonico commented Nov 4, 2026

Uh oh!

jonico commented Nov 11, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

jonico commented Oct 18, 2026

What it does

Validation

Uh oh!

jonico commented Oct 18, 2026

Uh oh!

timtebeek commented Oct 23, 2026

Uh oh!

timtebeek left a comment

Choose a reason for hiding this comment

Uh oh!

timtebeek Oct 23, 2026

Choose a reason for hiding this comment

Uh oh!

jonico Oct 23, 2026

Choose a reason for hiding this comment

Uh oh!

timtebeek Oct 23, 2026

Choose a reason for hiding this comment

Uh oh!

jonico Oct 23, 2026

Choose a reason for hiding this comment

Uh oh!

jonico commented Nov 4, 2026

Uh oh!

jonico commented Nov 11, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants